-
Notifications
You must be signed in to change notification settings - Fork 3k
Hive: Push Iceberg table property values to HMS table properties #2123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java
Outdated
Show resolved
Hide resolved
mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerNoScan.java
Outdated
Show resolved
Hide resolved
mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerNoScan.java
Outdated
Show resolved
Hide resolved
|
Build failure is due to the flaky Spark tests: |
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveMetastore.java
Show resolved
Hide resolved
25e457a to
6d630dc
Compare
|
What do you think about changing the title @marton-bod? I think the title below describes the change better: Thanks, |
|
Overall I agree, I think that describes the crux of the change better. I went with the original title because we also relax the initial set of properties we push down from HMS to Iceberg in this patch. However, until we have a pre/postAlter hook on the HiveMetaHook, this is mostly a one-way street indeed. |
9eb76fb to
e51d971
Compare
|
Thanks @marton-bod for the change! |
Iceberg table properties are the canonical source of truth HMS table properties should be maintained as much as possible to be in sync with the Iceberg table, but it can only happen on a best effort basis This PR makes the following changes: Ensures that all Iceberg table properties are propagated to the HMS table during HiveTableOperations commit All HMS table properties are pushed down to Iceberg as well during table creation (except for metadata location and spec props) Refactors the various property check assertions scattered throughout various test cases into a single property-focused unit test case What is left out and should be done in the future: Push property changes occurring via Hive DDL (ALTER TABLE SET TBLPROPERTIES) down to Iceberg as well. Currently this can't be done reliably because the HiveMetaHook interface only contains a preAlterTable method, but no commitAlterTable method. We'll need to extend this interface and include the change in an upcoming Hive upstream release. Author: Marton Bod <[email protected]> PR: apache/iceberg#2123 Backport Reason: To accomdate(I) for fix apache/iceberg#2328
Iceberg table properties are the canonical source of truth HMS table properties should be maintained as much as possible to be in sync with the Iceberg table, but it can only happen on a best effort basis This PR makes the following changes: Ensures that all Iceberg table properties are propagated to the HMS table during HiveTableOperations commit All HMS table properties are pushed down to Iceberg as well during table creation (except for metadata location and spec props) Refactors the various property check assertions scattered throughout various test cases into a single property-focused unit test case What is left out and should be done in the future: Push property changes occurring via Hive DDL (ALTER TABLE SET TBLPROPERTIES) down to Iceberg as well. Currently this can't be done reliably because the HiveMetaHook interface only contains a preAlterTable method, but no commitAlterTable method. We'll need to extend this interface and include the change in an upcoming Hive upstream release. Author: Marton Bod <[email protected]> PR: apache/iceberg#2123 Backport Reason: To accomdate(I) for fix apache/iceberg#2328
Iceberg table properties are the canonical source of truth HMS table properties should be maintained as much as possible to be in sync with the Iceberg table, but it can only happen on a best effort basis This PR makes the following changes: Ensures that all Iceberg table properties are propagated to the HMS table during HiveTableOperations commit All HMS table properties are pushed down to Iceberg as well during table creation (except for metadata location and spec props) Refactors the various property check assertions scattered throughout various test cases into a single property-focused unit test case What is left out and should be done in the future: Push property changes occurring via Hive DDL (ALTER TABLE SET TBLPROPERTIES) down to Iceberg as well. Currently this can't be done reliably because the HiveMetaHook interface only contains a preAlterTable method, but no commitAlterTable method. We'll need to extend this interface and include the change in an upcoming Hive upstream release. Author: Marton Bod <[email protected]> PR: apache/iceberg#2123 Backport Reason: To accomdate(I) for fix apache/iceberg#2328
As agreed with the community:
This PR makes the following changes:
HiveTableOperationscommitWhat is left out and should be done in the future:
ALTER TABLE SET TBLPROPERTIES) down to Iceberg as well. Currently this can't be done reliably because theHiveMetaHookinterface only contains apreAlterTablemethod, but nocommitAlterTablemethod. We'll need to extend this interface and include the change in an upcoming Hive upstream release.